Generate identifiers for unhydrated nodes #24665
Conversation
github-actions
bot
added
area: dds
There was a problem hiding this comment.
Pull Request Overview
This PR updates identifier handling for unhydrated TreeNodes by generating UUIDs on access instead of throwing errors, adjusts related tests, and updates documentation to reflect the new behavior.
- Introduce a global identifier allocator to generate and store UUIDs for unhydrated nodes in
objectNode.ts - Modify tests in
treeNodeApi.spec.tsto assert new default-ID behavior both before and after hydration - Update schemaFactory docs to note identifier presence rules and add a changeset entry
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| packages/dds/tree/src/test/simple-tree/api/treeNodeApi.spec.ts | Update tests to expect UUID allocation rather than errors for unhydrated nodes |
| packages/dds/tree/src/simple-tree/objectNode.ts | Add globalIdentifierAllocator and logic to allocate & store default IDs |
| packages/dds/tree/src/simple-tree/api/schemaFactory.ts | Revise doc comment to describe new identifier read/insertion rules |
| .changeset/public-snakes-fetch.md | Add release notes for implicit identifier generation |
Comments suppressed due to low confidence (1)
packages/dds/tree/src/test/simple-tree/api/treeNodeApi.spec.ts:1636
- [nitpick] The test description uses the term 'unread' which is unclear; consider renaming to 'read identifier' for consistency with the first test.
it("hydrated object with defaulted unread identifier field", () => {
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
| --- | ||
| TreeNodes now implicitly generate identifiers on access instead of throwing | ||
|
|
||
| Accessing a defaulted [identifier](https://fluidframework.com/docs/api/fluid-framework/schemafactory-class#identifier-property) on an [Unhydrated](https://fluidframework.com/docs/api/fluid-framework/unhydrated-typealias) `TreeNode` no longer throws a usage error. |
There was a problem hiding this comment.
Nit: a quick code example would probably be helpful (I'm pushing for more of these generally, as I think they make a huge difference)
Co-authored-by: Joshua Smithrud <54606601+Josmithr@users.noreply.github.com>
|
|
TreeIdentifierUtils.getShort's docs are the same as TreeNodeAPI.shortID except that it does not document what happens in the case where the id is a user provided string. What does it do in that case? Throw? Return undefined? |
It returns |
I have updated their docs, and TreeStatus.New which was also out of date. |
Description
Allocate UUIDs instead of erroring for unhydrated nodes.
Reviewer Guidance
The review process is outlined on this wiki page.